Skip to content

Conversation

@Geraldine-Edwards
Copy link

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

This PR contains the exercises within each step of the Sprint 5 Prep section

Questions

Happy to receive all feedback - especially regarding refactoring. Thanks!

@Geraldine-Edwards Geraldine-Edwards added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 4, 2025
@OracPrime OracPrime added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Dec 29, 2025
Copy link

@OracPrime OracPrime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally good code, with a few bits that need tidying up in the context of "good version control practice" and a few bits where I got picky because I'd have had nothing to say if I didn't get picky. Also through in an optional stretch exercise for fun

# ------------------------
# Q. What does double("22") return? Did it do what you expected? Why did it return the value it did?
# A. I expected the result to be 44 through coercion, like JavaScript.
# It returned the string repeated i.e. 2222 because Python is a strongly typed language and doesn't use coercion

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, Python isn't a strongly typed language, it's "duck typed". However it is as you point out more sensible in how it behaves than javascript.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OracPrime Thanks for the extra context here David :)
Every resource I went to said it was strongly typed! :D
I have done a bit more reading on the differences between dynamic, static, strong, and duck typing. For me, Python is strongly typed because it doesn’t automatically mix incompatible types like "22" + 2. Also it is dynamically typed, not statically typed, so variable types are checked at runtime and can change. I agree with you about it being duck typed though because even though it enforces type rules at runtime, it still lets objects work if the operation makes sense. :) (No wonder Python is popular!)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static typing, duck typing, dynamic typing, .... and the madness which is javascript!



# from typing import List, Set
def unique_count(nums: List[int]) -> int:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you've got 3 versions of this function here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OracPrime Haha, yes it is a bit confusing at first glance but this was some exercises from an article (https://dev.to/tusharsadhwani/the-comprehensive-guide-to-mypy-561m) that we had to complete.


reveal_type(counts) # The special magic reveal_type method -
# you should only use reveal_type to debug your code, and remove it when you're done debugging.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if it's "remove it when you're done debugging" it should probably never be checked in to github except commented out

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As before, everything in this file is from an article and we just had to work through the exercises to deepen understanding - its not any code/program that I have totally written myself. :D

print_favorite_color(me)


def print_favorite_color(person: Dict[str, str]) -> None: # added types to function definition

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again it's odd to have before and after methods both here and live. The second one will "win" so later code will call it, but maybe better to remove or comment out the before version


def print_favorite_color(person: Dict[str, str]) -> None: # added types to function definition
fav_color = person.get('favorite-color')
reveal_type(fav_color) # added this line here

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, don't leave reveal_type in for anything checked in.

# added integer type annotation and a return type of string for the output and renamed the function
# parameter to avoid shadowing issue with variable name in the outer scope
def format_pence_as_string(total_pence_amount: int) -> str:
if total_pence_amount < 100:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the account has a negative balance?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OracPrime Good question! :)
For this exercise we had to add the type annotations and fix the obvious bugs, but as you suggested when I ran this with negative account balances (so that the totals equalled positive and negative numbers) it handled both scenarios correcttly as part of the calculation.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have missed something, but I surely if total_pence_amount is -150 then you want £-1.50 whereas it will return -150p ??? I haven't run it, I'm just reading it.


imran = Person(name="Imran", children=[fatma, aisha])

def print_family_tree(person: Person) -> None:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're bored over the break an need a stretch objective, extend this to cope with grandchildren and great-grandchildren :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! I will look to doing this... :D

age = int(input("Please enter your age: "))
minimum_age = 18
if age < minimum_age:
raise ValueError("Age must be 18 or over.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to embed minimum_age into the error message, just in case it changes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, of course, avoiding the magic number scenario completely! :) I've amended this!

try:
name = (input("Please enter your first name: ")).strip()
if not name.isalpha():
raise ValueError("Name must contain only alphabetic characters.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tell that to Jean-Claude van Damme!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! Yes you are absolutely right! I need to always be taking into account the edge cases! Thanks David!

def get_full_name(self) -> str:
suffix = ""
if len(self.previous_last_names) > 0:
suffix = f" (née {self.previous_last_names[0]})"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're being picky, you should do a gender check here. For example Muhammad Ali, né Cassius Clay. You can probably skip this though!

@OracPrime OracPrime added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Dec 29, 2025
…. Embed minimum age variable into error message for accuracy
@Geraldine-Edwards Geraldine-Edwards added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jan 2, 2026
Copy link

@OracPrime OracPrime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

De'Wayne would probably object to your regex still, but I'm prepared to let that be his problem :)

# added integer type annotation and a return type of string for the output and renamed the function
# parameter to avoid shadowing issue with variable name in the outer scope
def format_pence_as_string(total_pence_amount: int) -> str:
if total_pence_amount < 100:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have missed something, but I surely if total_pence_amount is -150 then you want £-1.50 whereas it will return -150p ??? I haven't run it, I'm just reading it.

@OracPrime OracPrime removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jan 11, 2026
@OracPrime OracPrime added Reviewed Volunteer to add when completing a review with trainee action still to take. Complete Volunteer to add when work is complete and all review comments have been addressed. labels Jan 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants